refactor: keep walletId out of v1 activity payloads#117
Open
coreyphillips wants to merge 1 commit into
Open
Conversation
walletId was added directly to the activity models, which changed the v1 JSON schema without bumping the version. This restores v1 payload compatibility without introducing separate V1/V2 model structs. Default-wallet records now omit walletId from their JSON via skip_serializing_if, keeping the original v1 payload byte-identical. Records scoped to a non-default wallet keep walletId present, so the presence of the field acts as the version discriminator. Old v1 JSON without walletId still decodes, defaulting to the built-in wallet. Storage already scopes activity identity by (walletId, id) with composite primary keys, so lookup, list, search, tags, seen state, and transaction details stay wallet-scoped and never return mixed or duplicate entries for the same logical activity. Tests cover old v1 decode, v2 hardware-wallet activity creation, the wallet-scoped metadata payload rule, and mixed v1/v2 lookup, list, and search behavior for activities sharing the same raw id. Refs #112
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Resolves #112.
walletIdwas added directly to the activity models, which changed the v1 JSON schema without bumping the version. This restores v1 payload compatibility.After weighing the issue's literal request (separate
OnchainActivityV1/OnchainActivityV2andLightningActivityV1/LightningActivityV2structs), we kept the single-struct approach because it is strictly simpler here and meets every acceptance criterion. The reasoning:wallet_idis already a first-class column with composite primary key(wallet_id, id). A versionedenum { V1, V2 }only earns its keep when you persist serialized blobs and must decode an unknown shape, which this crate never does.wallet_idis a column, one canonical shape out.walletId, which already decodes via serde defaults.Approach
walletIdfrom their JSON viaskip_serializing_if, keeping the original v1 payload byte-identical.walletIdpresent, so the presence of the field is the version discriminator.walletIdstill decodes, defaulting to the built-in wallet.This is applied to
OnchainActivity,LightningActivity,ActivityTags,PreActivityMetadata, andTransactionDetails.Acceptance criteria
walletIdis not required on v1 activity payloads.walletId.walletIddecodes successfully.Tests
New tests cover old v1 decode, v2 hardware-wallet activity creation, the wallet-scoped metadata payload rule, and mixed v1/v2 lookup, list, and search behavior for activities sharing the same raw id.
cargo test modules::activity: 178 passed, 0 failed.cargo clippy: clean for the activity module.